Are you sure targetPlatform is right here, not hostPlatform?

I'd like to take this opportunity to ask the same question regarding the override here:

Both should be hostPlatform. I’ll submit a separate PR to fix the 11.0 stdenv definition and push an update to fix this one. Thanks for the feedback both of you.

I wonder if it might not be a good idea to export both versions of symbols to increase compatibility with the Linux version?

Both glibc and Musl use the iconv prefix. The reason for the libiconv prefix in GNU libiconv is to allow it to be used on systems that provide their own implementations in libc. However, on Darwin, the system implementation is GNU libiconv, and it’s being (or will be) replaced with this version.

As far as I can tell, (almost?) all Linux packages in nixpkgs use iconv from libc (glibc or Musl). It’s possible a Darwin app might link both the system and the separate libiconv, but I’d like to see an actual example before attempting to support that use case. I’d had to add to Darwin’s maintenance load to support something no one actually does.

Aha, makes sense. I just worry about having the same pkgs.libiconvReal with different behaviour across platforms, even though it's probably not going to come up. How would you feel about adding a forDarwinStdenv (or whatever) option to the package to conditionalize this logic on, and setting that in the pkgs.libiconv logic rather than changing pkgs.libiconvReal directly?

Maybe call it enableDarwinABICompat and default it to stdenv.hostplatform.isDarwin? If a package wants to do something wacky, it can override the compatibility and use both. I’ve pushed an updated branch with that change. Let me know what you think.

My suggestion is to call it e.g. useLibcAbi, default it to false, and then adjust all-packages.nix in this manner:

diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index cca76287015..69426ff28d1 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -22372,9 +22372,7 @@ with pkgs;
       then libcIconv (if stdenv.hostPlatform != stdenv.buildPlatform
         then libcCross
         else stdenv.cc.libc)
-    else if stdenv.hostPlatform.isDarwin
-      then darwin.libiconv
-    else libiconvReal;
+    else libiconvReal.override { useLibcAbi = true; };
 
   libcIconv = libc: let
     inherit (libc) pname version;

Advantages: one fewer Darwin-specific conditional, might fix issues on other platforms that currently use libiconvReal here with probably the wrong ABI for the use-case, keeps the comment above it accurate ("We also provide libiconvReal, which will always be a standalone libiconv, just in case you want it regardless of platform." – I think if we provide something called libiconvReal with this documentation it should match the API/ABI of the actual standalone libiconv, so if we do something other than this I think we need to change the name and comment. But it seems fine to me to just do the override in libiconv's definition, because it expresses what we want from it.)

As discussed on Matrix, I’d like to keep the change conservative and targeted at Darwin for now. Other platforms may have their own iconv implementation, so it would be a misnomer to call that the “libc ABI”. I also don’t want to propagate Darwin’s quirk in reexporting libcharset. At least for now, I’ll be keeping the name but defaulting it to false.

When it comes time to switch Darwin to this implementation, the definition of libiconv can be updated to libiconvReal.override { enableDarwinABICompat = true; }. That will require updates to a few other packages that reference darwin.libiconv directly and the implementation of a deprecation mechanism for the darwin attrset.

Thanks for the feedback! I’ve pushed a commit that defaults this to off.